feat: Reroll keys #2058
feat: Reroll keys #2058
Conversation
|
|
|
|
@hiroasano is attempting to deploy a commit to the Unkey Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughWalkthroughThe changes introduce a feature that enables users to reroll API keys directly from the key detail and settings pages in the dashboard. This includes the implementation of the Changes
Assessment against linked issues
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Thank you for following the naming conventions for pull request titles! 🙏 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (2)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/reroll-key.tsx (2)
178-195: Consider replacing the switch statement with an object lookup.The
getDateFromExpirationOptionfunction is correctly implemented. However, consider replacing the switch statement with an object lookup for better readability and performance:const EXPIRATION_OPTIONS_MAP: Record<string, number> = { '5m': 5 * 60 * 1000, '30m': 30 * 60 * 1000, '1h': 1 * 60 * 60 * 1000, '6h': 6 * 60 * 60 * 1000, '24h': 1 * 24 * 60 * 60 * 1000, '7d': 7 * 24 * 60 * 60 * 1000, }; function getDateFromExpirationOption(option: string) { const delay = EXPIRATION_OPTIONS_MAP[option] || 0; return new Date(Date.now() + delay); }
119-119: Simplify the conditional expression.The static analysis tool correctly suggests removing the unnecessary ternary operator. Simplify the code by directly assigning the result of the comparison:
- enabled: values.expiresIn === "now" ? false : true, + enabled: values.expiresIn !== "now",Tools
Biome
[error] 119-119: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/page.tsx (3 hunks)
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/reroll-key.tsx (1 hunks)
- apps/dashboard/lib/trpc/routers/index.ts (2 hunks)
- apps/dashboard/lib/trpc/routers/key/updateDeletedAt.ts (1 hunks)
Additional context used
Biome
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/reroll-key.tsx
[error] 119-119: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
Additional comments not posted (4)
apps/dashboard/lib/trpc/routers/index.ts (1)
69-69: LGTM!The code changes are approved.
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/reroll-key.tsx (1)
59-176: LGTM!The
RerollKeycomponent is well-implemented and follows best practices:
- It uses well-known libraries and components for form handling, validation, API calls, navigation, dialog state management, and UI.
- The form handling and validation are implemented correctly using
react-hook-formandzod.- The API calls are handled correctly with success and error cases using
trpc.- The navigation and dialog state management are implemented correctly using
next/navigationanduseState.- The UI is implemented using reusable components from
@/components/ui/*.- The loading and error states are handled correctly using
Loadingcomponent andtoastnotifications.Tools
Biome
[error] 119-119: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/page.tsx (2)
27-27: LGTM!The new import statement for the
RerollKeycomponent is syntactically correct and aligns with the feature implementation.
166-179: Excellent integration of the RerollKey component!The code segment properly integrates the
RerollKeycomponent within a newdivelement, alongside a link to the key settings. The necessary props (trigger,currentKey, andapiId) are correctly passed to theRerollKeycomponent.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- apps/dashboard/lib/trpc/routers/key/updateDeletedAt.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- apps/dashboard/lib/trpc/routers/key/updateDeletedAt.ts
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- apps/dashboard/lib/trpc/routers/key/updateDeletedAt.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- apps/dashboard/lib/trpc/routers/key/updateDeletedAt.ts
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/page.tsx
Outdated
Show resolved
Hide resolved
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/reroll-key.tsx
Outdated
Show resolved
Hide resolved
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/reroll-key.tsx
Outdated
Show resolved
Hide resolved
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/reroll-key.tsx
Outdated
Show resolved
Hide resolved
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/reroll-key.tsx
Outdated
Show resolved
Hide resolved
I believe you see the new key. it's not showing the old one because we're setting the |
Sorry, let me clarify, I meant the user is never shown the new secret, similar to how we show it when a new key is generated
In the modal, we should display the new key after they rerolled it. We don't need the curl command though |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/page.tsx (2 hunks)
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/page.tsx (2 hunks)
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/reroll-key.tsx (1 hunks)
- apps/dashboard/lib/trpc/routers/key/updateDeletedAt.ts (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/page.tsx
- apps/dashboard/lib/trpc/routers/key/updateDeletedAt.ts
Additional comments not posted (7)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/page.tsx (2)
17-17: LGTM!The code changes are approved.
71-71: LGTM!The code changes are approved.
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/reroll-key.tsx (5)
1-41: LGTM!The imports and component props are correctly used and typed.
53-59: LGTM!The form schema and component state are correctly defined and used.
69-90: LGTM!The tRPC mutations are correctly defined and used, and the
onSuccessandonErrorcallbacks are correctly used to handle the response.
92-112: LGTM!The
onSubmitfunction correctly creates a new key, calculates the expiration date, and expires the old key. The success and error cases are correctly handled using thetoastlibrary.
114-167: LGTM!The component rendering is correctly implemented using various UI components, the
useFormhook, and theonSubmitfunction.
ygorhiroshi
force-pushed
the
issue-2050
branch
from
876beb9
to
58e0e48
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/page.tsx (1 hunks)
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/page.tsx (3 hunks)
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/reroll-key.tsx (1 hunks)
- apps/dashboard/lib/trpc/routers/index.ts (2 hunks)
- apps/dashboard/lib/trpc/routers/key/updateDeletedAt.ts (1 hunks)
Files skipped from review as they are similar to previous changes (5)
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/page.tsx
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/page.tsx
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/reroll-key.tsx
- apps/dashboard/lib/trpc/routers/index.ts
- apps/dashboard/lib/trpc/routers/key/updateDeletedAt.ts
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/page.tsx (4 hunks)
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/reroll-key.tsx (1 hunks)
- apps/dashboard/lib/trpc/routers/key/create.ts (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/reroll-key.tsx
Additional context used
Biome
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/page.tsx
[error] 4-5: Some named imports are only used as types.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the transpilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
Additional comments not posted (4)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/page.tsx (3)
17-17: Import Statement forRerollKeyApprovedThe import statement correctly brings in the
RerollKeycomponent necessary for the new feature.
75-75: Verify Type Casting and Component PlacementThe
RerollKeycomponent is integrated into theSettingsPage. However, verify the following:
- The type casting of
apiKeytoKey & { roles: []}. Ensure this casting aligns with the expected types and does not introduce potential type errors.- The placement of the component within the JSX structure to ensure it is logical and consistent with UI design principles.
35-38: Updates to thewithObject ApprovedThe updates to the
withobject in the database query enhance security and identification features. However, verify the following:
- Ensure that these properties (
encrypted,identity,roles,permissions) are correctly used throughout the application.- Check for any potential performance impacts due to these additions.
apps/dashboard/lib/trpc/routers/key/create.ts (1)
36-36: Addition ofidentityIdApprovedThe new optional field
identityIdis correctly added to the input schema and reflected in the mutation logic. However, verify the following:
- Ensure that the optional nature of
identityIddoes not introduce any unintended side effects in the mutation logic.- Check for any potential issues related to data consistency or validation when
identityIdis not provided.Also applies to: 96-96
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/page.tsx (3 hunks)
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/reroll-confirmation-dialog.tsx (1 hunks)
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/reroll-key.tsx (1 hunks)
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/reroll-new-key-dialog.tsx (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/page.tsx
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/reroll-key.tsx
...oard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/reroll-confirmation-dialog.tsx
Outdated
Show resolved
Hide resolved
...dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/reroll-new-key-dialog.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/reroll-confirmation-dialog.tsx (1 hunks)
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/reroll-new-key-dialog.tsx (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/reroll-confirmation-dialog.tsx
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/reroll-new-key-dialog.tsx
ygorhiroshi
force-pushed
the
issue-2050
branch
from
deaf314
to
18fa6e7
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/page.tsx (1 hunks)
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/page.tsx (3 hunks)
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/reroll-confirmation-dialog.tsx (1 hunks)
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/reroll-key.tsx (1 hunks)
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/reroll-new-key-dialog.tsx (1 hunks)
- apps/dashboard/lib/trpc/routers/index.ts (2 hunks)
- apps/dashboard/lib/trpc/routers/key/create.ts (2 hunks)
- apps/dashboard/lib/trpc/routers/key/updateDeletedAt.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/page.tsx
Files skipped from review as they are similar to previous changes (6)
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/reroll-confirmation-dialog.tsx
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/reroll-key.tsx
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/reroll-new-key-dialog.tsx
- apps/dashboard/lib/trpc/routers/index.ts
- apps/dashboard/lib/trpc/routers/key/create.ts
- apps/dashboard/lib/trpc/routers/key/updateDeletedAt.ts
Additional comments not posted (3)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/page.tsx (3)
11-11: New component import:RerollKey.The import of the
RerollKeycomponent is correctly placed among other related component imports. This organization helps maintain clarity and consistency in the file structure.
80-84: Integration of theRerollKeycomponent.The
RerollKeycomponent is well-integrated into the JSX structure of theSettingsPage. The props passed to the component, includingapiId,apiKey, andlastUsed, are appropriate for its functionality. However, the casting ofkeytoKey & { roles: [] }should be reviewed to ensure it accurately reflects the expected data structure and does not introduce type mismatches.Run the following script to verify the type casting for accuracy:
Verification successful
Review the type casting for
apiKeyinRerollKey.The casting of
keytoKey & { roles: [] }in theRerollKeycomponent appears to be a workaround to meet the component's prop requirements. This approach is acceptable as long as the component can handle the emptyrolesarray appropriately. If further verification is needed, consider reviewing theschema.keysstructure to ensure the casting aligns with the expected data model.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the type casting for the `apiKey` prop in the `RerollKey` component. # Test: Search for the type definition of `Key` and its usage. Expect: Consistency with the casting. rg --type typescript -A 5 $'type Key'Length of output: 73
Script:
#!/bin/bash # Description: Verify the type casting for the `apiKey` prop in the `RerollKey` component. # Test: Search for the type definition of `Key` and its usage. Expect: Consistency with the casting. rg --type ts -A 5 $'type Key'Length of output: 5690
36-39: Enhanced security and identification features in thewithobject.The expansion of the
withobject to includeencrypted,identity,roles, andpermissionsis a positive change, likely supporting the newRerollKeyfunctionality. Ensure that these properties are properly utilized in the reroll process and that their integration is secure and efficient.Run the following script to verify the integration of these properties in the reroll functionality:
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
When rerolling a key with the selection time of "now"
It causes an error because you are sending null down for deleted at, and because at that point you already rerolled the key. It causes the original key to be valid and the new key to also be valid.
We need to ensure that if one part of the reroll fails, we handle this appropriately in the code and restore correctly.
For example:
- New key gets created, old key does not get deleted correctly. We need to ensure the new key is not valid, and the old key can be accessed.
- New key fails to be created do not start the processes of deleting a key.
ygorhiroshi
force-pushed
the
issue-2050
branch
from
18fa6e7
to
db4c22c
Compare
ygorhiroshi
force-pushed
the
issue-2050
branch
from
656cec0
to
f4ef0e9
Compare
|
@chronark @perkinsjr with this last commit I believe the PR is ready to be reviewed. |
There was a problem hiding this comment.
For keys that don't expire NOW you cannot set them to deleted. If you set the date to deleted it will cause them to be unusable.
We have a column named "expires" which would make more sense, as that will automatically set the deleted at, at the correct time.
Otherwise we would need to change the way the API works.
What does this PR do?
Implements a new feature to Reroll keys.
Fixes #2050
Type of change
How should this be tested?
Reroll Keyand a Dialog will show upReroll Keyaction buttonChecklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated
Here a link of the new feature recording
Reroll Key Dialog
Reroll Key Dialog - delay period options
Summary by CodeRabbit
Summary by CodeRabbit
New Features
RerollKeycomponent for regenerating API keys directly from the Settings Page.Bug Fixes